Skip to content

Location verify sweep + prune, and the scan volume-root fix - #196

Open
utof wants to merge 5 commits into
mainfrom
feat/location-verify-and-prune
Open

Location verify sweep + prune, and the scan volume-root fix#196
utof wants to merge 5 commits into
mainfrom
feat/location-verify-and-prune

Conversation

@utof

@utof utof commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Adds the ability to notice that an indexed file is gone, to see that in the UI, and to remove the stale entry — plus the scan fix that had to land first for any of it to mean anything.

Closes #191.

Why the scan fix is in here

Scanner::walk takes root and volume_root as distinct parameters; all three call sites passed the scan root for both, while record_mount stored the real mount point. So mount_path + relative_path did not reconstruct the file. Scanning /mnt/data/Videos recorded clip.mp4 instead of Videos/clip.mp4.

On a real library that was 417 of 419 rows pointing at nothing, surfacing as a misleading transcription failure (stat input: No such file or directory).

This blocks the rest of the PR rather than merely accompanying it: a verify sweep stats each reconstructed path and marks unreachable rows Missing, so run against a pre-fix catalogue it would mark almost every row Missing and hand the prune button a full-catalogue delete.

Note for existing databases: relative_path changes meaning, so a rescan inserts new rows rather than updating old ones. Fresh catalogues are unaffected.

What's new

perima verify / verify_locations walk locations on mounted volumes, stat each, write back changed statuses
perima prune / prune_missing_locations soft-delete rows marked missing
LocationStatusBadge + row dimming a missing file is visible in the file list
LibraryHealthPill "Check files" + a prune control that appears only when there is something to remove

Verify moves statuses both ways — a file that comes back returns to Active rather than being stuck Missing.

The safety property, and why it is structural

list_locations_for_verify INNER JOINs volume_mounts. A location whose volume is not mounted does not appear in the result set at all — it is not returned with a NULL path for the caller to remember to filter.

This is the one genuinely dangerous edge in the feature: the sweep marks everything it receives and cannot stat as Missing, so a single leaked row for an unplugged external drive becomes a false Missing, and prune then deletes a catalogue whose files are intact in a drawer. Making absence structural means no caller can get it wrong.

The count of excluded rows travels back with the rows (VerifyCandidates) rather than being an optional second query, and every surface prints it. A sweep that silently omits an unmounted drive reports "checked 78, no changes" over a 500-file library and reads as a clean bill of health.

That paid for itself immediately — see below.

The mount join is also scoped to the calling device. volume_mounts is keyed on (volume_id, machine_id); joining without that predicate pairs a local row with another machine's mount path. Three older queries omit it — #195, deliberately left alone so the trait-signature change lands as its own reviewable commit.

A pre-existing bug this surfaced

The first live run reported checked 0, skipped 78. Cause: CLI and desktop read device_id.txt from different directories, so they are different machines against one shared database.

$ perima volumes                                 MOUNT PATHS: (none)
$ PERIMA_CONFIG_DIR=<app_data_dir> perima volumes MOUNT PATHS: /home/vboxuser

Same DB, same machine, same volume. This already breaks a shipped command and is independent of this PR. #192 raised to priority/high with the evidence — device_id is the CRDT actor identity, so it is not a config-path annoyance.

Until #192 lands, perima verify from the CLI will report everything as skipped on a desktop-scanned library. The desktop button works.

Other decisions worth review

  • symlink_metadata, not metadata — a symlink whose target is gone still has an entry at the catalogued path.
  • Moved / Stale are left untouched when the file is present. The sweep only looked at existence, so it only writes the present/absent axis.
  • Status writes go through one batched transaction with one hlc. Per-row writes would open one BEGIN IMMEDIATE per changed file — the write-amplification shape behind SQLite lock-order inversion: concurrent Connection drops deadlock under WAL + multi-handle pattern #131. A sweep is one logical event; every row it touched saw the same filesystem state.
  • Prune soft-deletes. file_locations is CRDT-replicated; a hard delete has no merge representation and would resurrect on the next sync.
  • Prune does no filesystem check of its own — that would put stat() inside a write transaction. It is therefore exactly as trustworthy as the sweep before it, which is why both surfaces push toward running verify first.
  • CLI prune requires --yes; the desktop control is absent rather than disabled at zero, since a greyed "Remove 0 missing" invites a click to find out what it does.

Verification

Full just ci surface locally: clippy -D warnings, 426 Rust tests, 181 frontend tests, doctest, docs-coverage, typos, fmt, exec-bits, bun run build, eslint — all clean.

Two tests were confirmed to actually discriminate rather than trusting a green run:

  • the scan regression test fails without the fix with / + alpha.txt = /alpha.txt (not found);
  • the ipc-contract test catches a snake_case IPC arg with verify_locations: expected key "dryRun" (Rust param dry_run), api.ts sends [dry_run] — the T8 follow-up: standardise IPC arg case in api.ts (snake_case vs camelCase) #180 bug class, which shipped broken precisely because both spellings were assumed to work.

End-to-end on a real 78-file library: clean sweep reports no changes; hiding a file makes the next sweep mark exactly that row missing; prune without --yes refuses; --dry-run reports 1; --yes retires the row (78 -> 77 active + 1 soft-deleted); restoring the file makes the following sweep report it recovered.

cargo deny passes (advisories ok, bans ok, licenses ok, sources ok) but notes spin 0.9.8 is a yanked version via flume 0.12.0 — non-blocking, untouched here.

Related issues filed while building this

#192 (config/device-id divergence, raised to high), #193 (settings modal duplicates the Rust preset table), #194 (transcript.language stores provider free-text, not BCP-47), #195 (machine_id missing from three older joins).

🤖 Generated with Claude Code

utof and others added 5 commits August 1, 2026 20:19
`Scanner::walk` takes `root` and `volume_root` as distinct parameters —
the second is the prefix each file's `relative_path` is recorded
against. All three call sites passed the scan/watch root for both, while
`record_mount` stored the real mount point as `volume_mounts.mount_path`.
The two halves of the reconstruction therefore disagreed by exactly the
scan-root-minus-mount-point segment, and `mount_path + relative_path`
did not resolve to the file.

Scanning `/mnt/data/Videos` recorded `clip.mp4` instead of
`Videos/clip.mp4`, so the read path reconstructed `/mnt/data/clip.mp4`.
Observed on a real library as 417 of 419 rows pointing at nothing, which
surfaced downstream as a misleading transcription failure:

    Could not decode audio from source: stat input:
    No such file or directory (os error 2)

`relativize` itself was correct and well-tested; only its callers were
wrong.

Fixed at all three sites:

  - crates/app/src/scan.rs         — mount point, scan root in dry-run
  - crates/cli/src/cmd/watch.rs    — detected.mount_point
  - crates/desktop/src/commands.rs — detected.mount_point

The watch sites matter beyond path reconstruction: watcher-emitted
`FileEvent::Deleted` is currently the only producer of
`LocationStatus::Missing`, so it was keyed on a different root than the
one scan wrote.

Safety: `detect_volume` selects its mount by `canonical.starts_with(mp)`
with longest-prefix-wins, so the mount point is guaranteed to be an
ancestor of the scan root. `relativize`'s `strip_prefix` cannot begin
failing as a result — it was succeeding against the wrong prefix.

WHY the suite missed this: every scan test uses a `tempfile` root, and a
temp dir sits under a mount rather than being one, so `walk(root, root)`
was correct in the fixture and wrong in production. No test joined
`mount_path` and `relative_path` back together, which is the only place
the disagreement is observable.

The new regression test asserts that reconstruction — the same join the
read path performs — and was verified to fail before this fix with
`/ + alpha.txt = /alpha.txt (not found)`. It also fails loudly if the
fixture root is ever its own mount point, rather than passing vacuously.

`scan_persists_metadata_rows_for_images` matched `relative_path =
'red.png'` exactly, encoding the old behaviour; it now matches the
basename, with a count guard so the lookup cannot silently become
ambiguous.

Note for existing databases: `relative_path` changes meaning, so a
rescan inserts new rows rather than updating old ones. Fresh catalogues
are unaffected.

Closes #191

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`beforeDevCommand` runs with `"wait": false`, so Tauri spawns
`bun run dev` without reaping it, and `bun` spawns `vite` as a
grandchild. When Tauri exits, that grandchild survives holding port
5173. Because `devUrl` pins that exact port, the next `just dev` fails
outright rather than reusing or relocating:

    error when starting dev server:
    Error: Port 5173 is already in use
    Error The "beforeDevCommand" terminated with a non-zero status code.

Every second launch failed until the stray process was killed by hand.

The pattern is the repo-local vite binary path, so it cannot match a dev
server belonging to another project on the same machine.

WHY `[v]ite` rather than `vite`: `pkill -f` matches the full command line
of every process, including the shell running the recipe — whose argv
contains the pattern verbatim. Spelled plainly, pkill kills its own shell
and the recipe dies with exit 144 before reaching the `tauri dev` line.
Verified both ways against a scratch justfile: `vite` reproduces the
self-kill, `[v]ite` prints the following line and exits 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scanning records where files were; nothing afterwards noticed when one
went away. The filesystem watcher only sees deletions that happen while
it is running, so a file removed with the app closed stayed `Active` in
the catalogue forever. `perima verify` walks every location on a mounted
volume, stats it, and writes back the statuses that changed — both
directions, so a file that comes back returns to `Active` rather than
being stuck `Missing`.

This has to exist before any prune: deleting rows marked `Missing` is
only as trustworthy as whatever marked them.

## The unmounted-volume rule is structural, not documented

`list_locations_for_verify` INNER JOINs `volume_mounts`, so a location
whose volume is not mounted does not appear in the result set at all —
it is not returned with a NULL path for the caller to remember to
filter. The sweep marks everything it receives and cannot stat as
`Missing`, so one leaked row for an unplugged external drive becomes a
false `Missing`, and a prune then deletes a catalogue whose files are
intact in a drawer. Making absence structural means no caller can get
this wrong.

The count of excluded rows travels back with the rows themselves
(`VerifyCandidates`) rather than being an optional second query. A sweep
that silently omits an unmounted drive reports "checked 78, no changes"
over a 500-file library and reads as a clean bill of health; carrying
the number the caller may NOT conclude anything about makes an honest
report the path of least resistance. `VerifyReport::is_complete()` is
the single predicate a destructive caller should gate on.

## machine_id

The mount join is scoped to the calling device. `volume_mounts` is keyed
on `(volume_id, machine_id)`, and joining without that predicate pairs a
local row with another computer's mount path — the sweep would then stat
paths that mean nothing here and mark the whole local catalogue
`Missing`. Three older queries in `file_repo.rs` omit the predicate;
that is #195, left alone here so the trait-signature change lands as its
own reviewable commit.

## Other decisions

- `symlink_metadata`, not `metadata`: a symlink whose target is gone
  still has an entry at the catalogued path. Whether the target is
  healthy is the hashing path's question.
- `Moved` and `Stale` are left untouched when the file is present. This
  sweep only looked at existence, so it only writes the present/absent
  axis; downgrading either to `Active` would discard information it did
  not earn.
- Status writes go through one batched transaction. Per-row writes would
  open one `BEGIN IMMEDIATE` per changed file — the write-amplification
  shape that provoked the SQLite lock-order inversion in #131. One `hlc`
  covers the batch: a sweep is one logical event, and every row it
  touched observed the same filesystem state.
- A cancelled sweep writes nothing and reports `completed: false`.

Verified end-to-end against a real 78-file library: a clean sweep reports
no changes; hiding a file makes the next sweep mark exactly that row
`missing`; restoring it makes the following sweep report it recovered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds `perima prune` plus the `verify_locations`, `count_missing_locations`
and `prune_missing_locations` Tauri commands, so both shells can act on
what a verify sweep found.

Prune soft-deletes `file_locations` rows whose status is `missing`. It
performs no filesystem check of its own — deciding what is missing needs
a `stat()` per file, which must not happen inside a write transaction
holding the write lock. That split means a prune is exactly as
trustworthy as the sweep before it, which is why both surfaces push the
user toward running `verify` first.

Soft-delete rather than `DELETE FROM`: `file_locations` is CRDT-
replicated, a hard delete has no merge representation, and the row would
resurrect on the next sync from any peer that still holds it. Same
convention as the `update_location_path` collision branch.

## Confirmation

The CLI requires an explicit `--yes` for a live run and otherwise prints
the count with a pointer to `--dry-run`. The desktop hides the prune
control entirely when nothing is missing rather than showing it disabled
— a greyed "Remove 0 missing" invites a click to find out what it does —
and requires a second click that restates the number.

Prune is also a no-op when the count is zero: an empty destructive write
still takes the write lock and still emits an invalidation the frontend
would act on.

`verify_locations` runs on `spawn_blocking`. The sweep issues one
`symlink_metadata` per catalogued file, and a large library would
otherwise stall the async runtime thread the Tauri command is polled on.

## IPC arg names

All three commands take their arguments under the camelCase keys Tauri
converts to, and the `ipc-contract` test covers them. Confirmed the test
actually discriminates by flipping `dryRun` to `dry_run` and watching it
fail with the exact diagnostic, rather than trusting a green run:

    verify_locations: expected key "dryRun" (Rust param `dry_run`),
    api.ts sends [dry_run]

That is the #180 bug class, which shipped broken precisely because it
was assumed both spellings worked.

Verified end-to-end on a real library: verify marks one hidden file
missing, prune without `--yes` refuses, `--dry-run` reports 1, `--yes`
retires the row (78 active -> 77 active + 1 soft-deleted).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The catalogue has tracked whether a file is still on disk since v1, and
`payloads.rs` has been shipping that status to the frontend the whole
time — where `FileTable` rendered it as bare lowercase text in the fifth
of six columns, visually identical to every other cell. The information
was present and invisible.

`LocationStatusBadge` gives the abnormal states a treatment and leaves
`active` muted. Colouring the normal case too would drown the one row
that matters, since almost every row is active. Missing rows also drop
to 50% opacity across the whole row: the status column is easy to miss
when scanning by filename, and desaturating the row makes "not on disk"
legible peripherally while the badge carries the precise verdict.
Opacity only — the row stays selectable so it can be inspected.

Meaning is carried in text as well as colour (`sr-only` description per
state), so the signal survives assistive tech and colour blindness. An
unrecognised status renders neutrally rather than blanking the cell —
`status` is a plain text column in SQLite and a future backend variant
must not break the table.

`LibraryHealthPill` in the status bar exposes the two actions. They stay
separate buttons rather than one "clean up": verify is safe and
repeatable, prune deletes rows, and fusing them would mean a single
click both decides what is missing and acts on it with no moment to look
at the number. The prune control is absent, not disabled, when nothing
is missing.

The verify notification always states `skipped_unmounted` when non-zero.
A sweep that could not see an unmounted drive would otherwise report
"checked 0 · no changes" and read as a healthy library — which is
exactly what happened on the first live run here, where CLI and desktop
turned out to hold different device ids (#192) and every location was
skipped. That surfaced only because the count is mandatory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@utof

utof commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

⚠️ Hold this merge — see #197.

CI is green across all five jobs, but a cargo-mutants audit of this branch found a real defect in it: VerifyReport::is_complete(), documented in three places in this PR as the gate a destructive caller must check before pruning, has zero callers outside its own unit tests.

What actually gates the prune as this branch stands: --yes in the CLI, nothing in the Tauri command, and a raw useMissingCount() DB count behind the desktop Remove button — none of which have any relationship to whether a sweep ran or whether it was complete.

Also in this branch: the WHY block in verify.rs justifying its hand-written test stub claims "the adapter's SQL is exercised separately". That was false when written — list_locations_for_verify, update_location_statuses, count_missing_locations and soft_delete_missing_locations had no tests at any layer. The audit added 10, each proven to discriminate.

The concrete danger needs no unplugged drive: on a pre-#191 catalogue every path reconstructs wrongly (measured at 417 of 419 rows on a real library), so a complete, fully-mounted sweep marks the whole library Missing and the UI offers to delete it. is_complete() would have returned true for that sweep.

#197 has the full analysis, the disproven unmount hypothesis, and the proposed three-layer fix. A work-in-progress diff exists locally but is uncommitted and not yet compile-verified, so it is deliberately not pushed here.

Everything else in this PR stands — the #191 scan fix, the sweep's unmounted-volume safety, and the UI work are unaffected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scan records relative_path against the scan root, not the volume root

1 participant